mGCA: Validate const literal against expected type#152001
mGCA: Validate const literal against expected type#152001rust-bors[bot] merged 3 commits intorust-lang:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
very cool :) thanks for working on this |
This comment has been minimized.
This comment has been minimized.
…=oli-obk Dont strip const blocks in array lengths r? oli-obk mGCA now handles const blocks by *always* handling them during `lower_expr_to_const_arg_direct` instead of *sometimes* stripping them out at parse time. This is just generally a lot clearer/nicer but also means parsing isn't lossy which is just straight up wrong. We now use `MgcaDisambiguation::Direct` for const blocks because we "directly" represent a const block as `hir::ConstArgKind::Anon` :> The only time that an anon const for const generics uses `MgcaDisambiguation::AnonConst` is for unbraced literals. Once we properly support literals in `hir::ConstArgKind` (see rust-lang#152139 rust-lang#152001) then `MgcaDisambiguation` can be renamed to `AnonConstKind` with `TypeSystem` and `NonTypeSystem` variants. We can also get rid of `mgca_direct_lit_hack`. I expect this to be a very nice cleanup :) Fixes rust-lang/rustfmt#6788 The diff relating to passing spans around is to avoid a bunch of mGCA diagnostics changing from `const {}` to `{}`. I'm not entirely sure why this was happening. cc @rust-lang/rustfmt How do I run the tests in the rustfmt repo from here? `x test rustfmt` only seems to run like 100 tests and doesn't result in a `target/issue-6788.rs` getting created. I've verified locally that this formats correctly though
…=oli-obk Dont strip const blocks in array lengths r? oli-obk mGCA now handles const blocks by *always* handling them during `lower_expr_to_const_arg_direct` instead of *sometimes* stripping them out at parse time. This is just generally a lot clearer/nicer but also means parsing isn't lossy which is just straight up wrong. We now use `MgcaDisambiguation::Direct` for const blocks because we "directly" represent a const block as `hir::ConstArgKind::Anon` :> The only time that an anon const for const generics uses `MgcaDisambiguation::AnonConst` is for unbraced literals. Once we properly support literals in `hir::ConstArgKind` (see rust-lang#152139 rust-lang#152001) then `MgcaDisambiguation` can be renamed to `AnonConstKind` with `TypeSystem` and `NonTypeSystem` variants. We can also get rid of `mgca_direct_lit_hack`. I expect this to be a very nice cleanup :) Fixes rust-lang/rustfmt#6788 The diff relating to passing spans around is to avoid a bunch of mGCA diagnostics changing from `const {}` to `{}`. I'm not entirely sure why this was happening. cc @rust-lang/rustfmt How do I run the tests in the rustfmt repo from here? `x test rustfmt` only seems to run like 100 tests and doesn't result in a `target/issue-6788.rs` getting created. I've verified locally that this formats correctly though
…=oli-obk Dont strip const blocks in array lengths r? oli-obk mGCA now handles const blocks by *always* handling them during `lower_expr_to_const_arg_direct` instead of *sometimes* stripping them out at parse time. This is just generally a lot clearer/nicer but also means parsing isn't lossy which is just straight up wrong. We now use `MgcaDisambiguation::Direct` for const blocks because we "directly" represent a const block as `hir::ConstArgKind::Anon` :> The only time that an anon const for const generics uses `MgcaDisambiguation::AnonConst` is for unbraced literals. Once we properly support literals in `hir::ConstArgKind` (see rust-lang#152139 rust-lang#152001) then `MgcaDisambiguation` can be renamed to `AnonConstKind` with `TypeSystem` and `NonTypeSystem` variants. We can also get rid of `mgca_direct_lit_hack`. I expect this to be a very nice cleanup :) Fixes rust-lang/rustfmt#6788 The diff relating to passing spans around is to avoid a bunch of mGCA diagnostics changing from `const {}` to `{}`. I'm not entirely sure why this was happening. cc @rust-lang/rustfmt How do I run the tests in the rustfmt repo from here? `x test rustfmt` only seems to run like 100 tests and doesn't result in a `target/issue-6788.rs` getting created. I've verified locally that this formats correctly though
…=oli-obk Dont strip const blocks in array lengths r? oli-obk mGCA now handles const blocks by *always* handling them during `lower_expr_to_const_arg_direct` instead of *sometimes* stripping them out at parse time. This is just generally a lot clearer/nicer but also means parsing isn't lossy which is just straight up wrong. We now use `MgcaDisambiguation::Direct` for const blocks because we "directly" represent a const block as `hir::ConstArgKind::Anon` :> The only time that an anon const for const generics uses `MgcaDisambiguation::AnonConst` is for unbraced literals. Once we properly support literals in `hir::ConstArgKind` (see rust-lang#152139 rust-lang#152001) then `MgcaDisambiguation` can be renamed to `AnonConstKind` with `TypeSystem` and `NonTypeSystem` variants. We can also get rid of `mgca_direct_lit_hack`. I expect this to be a very nice cleanup :) Fixes rust-lang/rustfmt#6788 The diff relating to passing spans around is to avoid a bunch of mGCA diagnostics changing from `const {}` to `{}`. I'm not entirely sure why this was happening. cc @rust-lang/rustfmt How do I run the tests in the rustfmt repo from here? `x test rustfmt` only seems to run like 100 tests and doesn't result in a `target/issue-6788.rs` getting created. I've verified locally that this formats correctly though
Rollup merge of #152234 - BoxyUwU:dont_strip_const_blocks, r=oli-obk Dont strip const blocks in array lengths r? oli-obk mGCA now handles const blocks by *always* handling them during `lower_expr_to_const_arg_direct` instead of *sometimes* stripping them out at parse time. This is just generally a lot clearer/nicer but also means parsing isn't lossy which is just straight up wrong. We now use `MgcaDisambiguation::Direct` for const blocks because we "directly" represent a const block as `hir::ConstArgKind::Anon` :> The only time that an anon const for const generics uses `MgcaDisambiguation::AnonConst` is for unbraced literals. Once we properly support literals in `hir::ConstArgKind` (see #152139 #152001) then `MgcaDisambiguation` can be renamed to `AnonConstKind` with `TypeSystem` and `NonTypeSystem` variants. We can also get rid of `mgca_direct_lit_hack`. I expect this to be a very nice cleanup :) Fixes rust-lang/rustfmt#6788 The diff relating to passing spans around is to avoid a bunch of mGCA diagnostics changing from `const {}` to `{}`. I'm not entirely sure why this was happening. cc @rust-lang/rustfmt How do I run the tests in the rustfmt repo from here? `x test rustfmt` only seems to run like 100 tests and doesn't result in a `target/issue-6788.rs` getting created. I've verified locally that this formats correctly though
e08830e to
cd5b323
Compare
This comment has been minimized.
This comment has been minimized.
|
Haha, ICE occurs in other tests... |
ac69206 to
ca77eab
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
In ten crash tests, the ICE disappeared; however, in tests/crashes/117460.rs, tests/crashes/114212-2.rs and tests/crashes/114212.rs, adding the main function caused the ICE to occur again.
| @@ -0,0 +1,23 @@ | |||
| //! Regression test for <https://github.com/rust-lang/rust/issues/116554> | |||
There was a problem hiding this comment.
This issue was closed; is there a need to add further testing?
There was a problem hiding this comment.
unsure, I will look over all the GCE tests (outside of this PR) at some point soon and see which ones are worth keeping
This comment has been minimized.
This comment has been minimized.
75e7e18 to
2b5c8b7
Compare
| pub neg: bool, | ||
| } | ||
|
|
||
| pub fn const_lit_matches_ty<'tcx>( |
There was a problem hiding this comment.
This does not seem like a great place for this function -- it doesn't really have anything to do with the interpreter, does it?
I guess you put it next to LitToConstInput, but that type also doesn't belong here either IMO. It has very little to do with everything else in this module.
There was a problem hiding this comment.
Oh, yes, I placed it here to place it in the same place as LitToConstInput.
I'll look for something more meaningful! LitToConstInput too.
There was a problem hiding this comment.
I created rustc_middle/src/ty/consts/lit.rs and moved it there. Does it look okay?
There was a problem hiding this comment.
that seems ok to me, we might wind up getting more logic for this as mGCA progresses so seems nice to have a module 🤔
5be3fdc to
4f93ae7
Compare
Co-authored-by: Boxy <rust@boxyuwu.dev>
…eneric_const_exprs/lit_type_mismatch.rs
Relocate LitToConstInput and const_lit_matches_ty from mir::interpret to ty::consts::lit
4f93ae7 to
01d48c6
Compare
| foo::<{ Foo { field: -1_usize } }>(); | ||
| //~^ ERROR: type annotations needed for the literal | ||
| foo::<{ Foo { field: { -1_usize } } }>(); | ||
| //~^ ERROR: complex const arguments must be placed inside of a `const` block |
There was a problem hiding this comment.
This error message implies we handle negated literals incorrectly during ast lowering I think. lower_expr_to_const_arg_direct is presumably not handling { -1_usize } even though it should (I think probably there is no reason for the arm for blocks to match on what kind of expr is being wrapped).
If you'd be interested, once this PR lands would you be interested in fixing that and #152246 in a followup PR?
There was a problem hiding this comment.
I think we probably also handle fn foo<const F: &str>() { foo::<{ -"hi" }>(); } wrong (i.e. we accept it?). I think this is also a problem with ast lowering and we should be checking there that the literal is an integer. Could also do that in a followup PR if you like?
There was a problem hiding this comment.
Yes, I'd be happy to work on that in a follow-up PR! I'll take a closer look at the AST lowering behavior you mentioned.
|
@bors r+ |
mGCA: Validate const literal against expected type close: rust-lang#151625 close: rust-lang#150983 also fix: rust-lang#133966 (moved crashes test)
mGCA: Validate const literal against expected type close: rust-lang#151625 close: rust-lang#150983 also fix: rust-lang#133966 (moved crashes test)
…uwer Rollup of 7 pull requests Successful merges: - #152001 (mGCA: Validate const literal against expected type) - #152120 (Don't ICE on layout error in vtable computation) - #152512 (core: Implement feature `float_exact_integer_constants`) - #152531 (`proc_macro::bridge`: simplify `ExecutionStrategy` and `DispatcherTrait`) - #152577 (Port #[rustc_proc_macro_decls] to the new attribute parser.) - #152590 (DepGraphQuery: correctly skip adding edges with not-yet-added nodes) - #152612 (Rename `inline_fluent!` to `msg!`)
…uwer Rollup of 8 pull requests Successful merges: - #152618 (stdarch subtree update) - #152001 (mGCA: Validate const literal against expected type) - #152120 (Don't ICE on layout error in vtable computation) - #152531 (`proc_macro::bridge`: simplify `ExecutionStrategy` and `DispatcherTrait`) - #152577 (Port #[rustc_proc_macro_decls] to the new attribute parser.) - #152570 (Port #[rustc_test_marker] to the attribute parser) - #152590 (DepGraphQuery: correctly skip adding edges with not-yet-added nodes) - #152612 (Rename `inline_fluent!` to `msg!`)
close: #151625
close: #150983
also fix: #133966 (moved crashes test)